Skip to content

Inorder implementation#99

Open
cMancio00 wants to merge 25 commits intokaste:masterfrom
cMancio00:inorder-implementation
Open

Inorder implementation#99
cMancio00 wants to merge 25 commits intokaste:masterfrom
cMancio00:inorder-implementation

Conversation

@cMancio00
Copy link

@cMancio00 cMancio00 commented Sep 23, 2025

This is the PR for #98.

The idea is to make Mock observable so we can register ordered invocations from different mocks.
The mocking_registry helps to get the mocks for comparing.

Features

  • Mocks should be observable from an InOrder object to register the order of invocations
  • Trying to observe the same Mock more than once should be fine
  • Should work with specs and no-specs Mocks
    Should work with spy objects (Maybe in an other PR)
  • Mocks should be detached automatically with a context manager and also when the Queue call is empty
  • Verification errors should be meaningful

@kaste Feel free to add elements to this list

@cMancio00
Copy link
Author

cMancio00 commented Sep 23, 2025

I should not use @override annotation since is not supported before Python 3.12 and also use mypy locally first.

@kaste
Copy link
Owner

kaste commented Sep 24, 2025

Very promising.

For the tests:

  • Don't do the friends things if it only hides the actual usage of the mocks in the tests.

  • Maybe even the setup is limiting here. Sometimes we test a specced mock, sometimes
    not.

cat = mock()
in_order = InOrder([cat])
cat.meow("HI")
cat.meow("HO")

a)
in_order.verify(cat).meow("HO")  # raises

b) 
in_order.verify(cat).meow("HI")  # passes
in_order.verify(cat).meow("HO")  # passes
in_order.verify(cat).meow("HO")  # raises, no?

b1) 
in_order.verify(cat).meow("HI")  # passes
in_order.verify(cat).meow("HO")  # passes
in_order.verify(cat).meow("HI")  # raises

c)
in_order.verify(cat).meow(...)  # passes
in_order.verify(cat).meow(...)  # raises

d)
in_order.verify(cat).bark(...)  # raises
  • implement context manager to "detach" automatically.
cat = mock()
with InOrder([cat]) as in_order:
    cat.meow("HI")
cat.meow("HO")

...

  • Check the actual error messages in the tests. The most important part is to
    have good error messages. E.g. user expected bark(...) but next invocation was
    actually meow("HI").

@kaste
Copy link
Owner

kaste commented Sep 24, 2025

(I usually do the tests first. E.g what behavior do I even expect for all the cases. Do they all make sense in concerto.)

@kaste
Copy link
Owner

kaste commented Sep 24, 2025

a)
probably valid and we have two separate "lines" of recorded executions
with InOrder(cat) as a, InOrder(dog) as b:

b)
a = InOrder(cat)
b = InOrder(cat)   # raises? already registered

@cMancio00
Copy link
Author

  • Don't do the friends things if it only hides the actual usage of the mocks in the tests.

Okay, I'edit the set up to use a function that calls both the interfaces

  • Maybe even the setup is limiting here. Sometimes we test a specced mock, sometimes
    not.

I'll also test non specs mocks

(I usually do the tests first. E.g what behavior do I even expect for all the cases. Do they all make sense in concerto.)

I'll try to implement it in TDD

a = InOrder(cat)
b = InOrder(cat) # raises? already registered

I thought that if we use a set as a container for observed subjects we can not have problems with already registered objects and the scope of an InOrder object is just in a single test case


def remember(self, invocation):
self.invocations.append(invocation)
self.notify()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it reads much better when we could do, for each observer. oberserver.notify(invocation). This is semantically equivalent to: for each observer. observer.remember(invocation). As if the invocations here are a specialized variant of it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look in to it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still: for each observer. observer.remember(invocation)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

☝️

@kaste
Copy link
Owner

kaste commented Sep 27, 2025

cat = mock()
dog = mock()

with InOrder(cat) as in_order:
    dog.bark()
    cat.meow()

in_order.verify(dog).bar()  #  raises

@kaste
Copy link
Owner

kaste commented Sep 27, 2025

a = InOrder(cat)
b = InOrder(cat) # raises? already registered

Yeah, that should probably not raise. Nesting should ideally work, we need to ensure instead that nesting works.

@cMancio00
Copy link
Author

cMancio00 commented Sep 28, 2025

Added a testing library to express better assertions with containers...I'm working in the various steps
@kaste Isn't sufficient to use

rye add --dev assertpy

@kaste
Copy link
Owner

kaste commented Sep 29, 2025

Generally, this project predates any of the modern tools, like rye, uv etc.

Don't do these distracting things here. They don't help you implement the actual thing.

For this new feature, just use pytest and plain asserts. You don't even need to subclass unittest.TestCase. Look at numpy_test.py as an example. Do not write test around in_order.mocks and in_order.ordered_invocations (=remembered_invocations). These are likely implementation details. Users shouldn't need those. Again: Try to use dumb mocks. Inline everything, don't be DRY here.

Already think about using InOrder(*mocks) to support the shorter and easier to write InOrder(mock()).

@cMancio00
Copy link
Author

Generally, this project predates any of the modern tools, like rye, uv etc.

Don't do these distracting things here. They don't help you implement the actual thing.

For this new feature, just use pytest and plain asserts. You don't even need to subclass unittest.TestCase. Look at numpy_test.py as an example. Do not write test around in_order.mocks and in_order.ordered_invocations (=remembered_invocations). These are likely implementation details. Users shouldn't need those. Again: Try to use dumb mocks. Inline everything, don't be DRY here.

Already think about using InOrder(*mocks) to support the shorter and easier to write InOrder(mock()).

I don't get it, Unit Tests should help me implementing the feature and serves other developers as a sort of documentation. Users should not reads our tests, but the documentation and the example in it (like doc tests). (In an extremis we can provide E2E tests with behave to get the high level functionality of a feature.
The assertion library that I wanted was to have better readability in the test cases and to have functions to have better tests while working with containers.

Anyway the repo is your so I'll do what you want. I'll remove the dependency, use dumb mock, inline everything (no setup like xUnit) and write the tests just with documentation purpuse

@cMancio00
Copy link
Author

@kaste Hello, what do you think?
I still need to implement the context manager and the unstub.

Do you think that we should support a workflow like this?:

  a = mock()
  b = mock()

  when(a).method().thenReturn("Calling a")
  when(b).other_method().thenReturn("Calling b")

  in_order: InOrder = InOrder([a,b])
  a.method()
  b.other_method()
  a.method()

  in_order.verify(a).method()
  in_order.verify(b).other_method()
  in_order.verify(a).method()

For now it is not supported.

Best regards

@kaste
Copy link
Owner

kaste commented Oct 5, 2025

Isn't that the meat of the re-implementation of InOrder?

cat = mock()
dog = mock()

when InOrder(cat, dog) as in_order:
    cat.meow()
    dog.bark()
    cat.meow()

in_order.verify(cat).meow()
in_order.verify(dof).bark()
in_order.verify(cat).meow()

The old InOrder supported only one mock, and your implementation supports proper cross-mock interactions?

@kaste kaste force-pushed the master branch 5 times, most recently from 26ca517 to 70a7c8a Compare October 5, 2025 11:26
@kaste
Copy link
Owner

kaste commented Oct 5, 2025

I pushed master to update to modern tooling (uv) and generally towards v2. Please try a rebase. E.g. RememberedInvocation | RememberedProxyInvocation is now invocation.RealInvocation.

@cMancio00
Copy link
Author

The old InOrder supported only one mock, and your implementation supports proper cross-mock interactions?

My implementation supports cross-mock interaction like

in_order.verify(cat).meow()
in_order.verify(dof).bark()

but doesn't yet supports an interaction of this type:

in_order.verify(cat).meow()
in_order.verify(dof).bark()
in_order.verify(cat).meow()

Where a mock method is called twice (or more) in different orders.
But this is easy to fix (I think). After checking that the specific instance of a mock is called I delegate to the original implementation of verify with times=1. I think changing it to atleast=1 solves the issue.

I pushed master to update to modern tooling (uv) and generally towards v2. Please try a rebase. E.g. RememberedInvocation | RememberedProxyInvocation is now invocation.RealInvocation.

I'll looking into that

@cMancio00 cMancio00 marked this pull request as ready for review October 5, 2025 16:00
@cMancio00
Copy link
Author

@kaste I' happy with the implementation and tests. Let me know if you need any changes or some documentation to put in the docs with examples (and how you wanted).
Best regards

P.S.
I would like to help in an other PR with docs (even for all the project) and "fix" spy objects. If you also need help with the CI let me know.

def mocks(self):
return self._mocks

def update(self, subject: Mock) -> None:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> remember(self, invocation: RealInvocation) -> None

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you haven't done this?

:param subject: subject to be added to the list of ordered invocation
"""
self.ordered_invocations.append(
(subject, subject.invocations[-1])
Copy link
Owner

@kaste kaste Oct 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subject would have been too generic anyway, but subject.invocations[-1] is an implementation detail. Just pass the invocation in. self.invocations: deque[RealInvocations]

Comment on lines 42 to 45
RealInvocation = Union[
invocation.RememberedInvocation,
invocation.RememberedProxyInvocation
]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RealInvocation is actually defined in invocation.py

Comment on lines 29 to 30
a = mock()
b = mock()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use a and b as names. Use cat and dog or some other names.

@kaste
Copy link
Owner

kaste commented Nov 18, 2025

Difficult to think this through... The standard verify should be supported at least.

@kaste kaste force-pushed the inorder-implementation branch from 66f219a to 07df123 Compare November 18, 2025 12:25
@kaste kaste force-pushed the inorder-implementation branch from 07df123 to cfe9508 Compare January 19, 2026 15:09
@kaste kaste force-pushed the inorder-implementation branch from cfe9508 to 6d5ec3e Compare February 16, 2026 23:48
@kaste kaste force-pushed the inorder-implementation branch from c38f692 to d5dd049 Compare February 16, 2026 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants